-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Mine #6569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mine #6569
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and found several issues that need attention before this can be merged.
| import { BaseProvider } from "./base-provider" | ||
|
|
||
| // OAuth2 Configuration (from Cline implementation) | ||
| const OAUTH_CLIENT_ID = "681255809395-oo8ft2oprdrnp9e3aqf6av3hmdib135j.apps.googleusercontent.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 Critical Security Issue: These OAuth credentials should never be hardcoded in the source code. This poses a serious security risk as anyone can use these credentials.
Consider:
- Moving these to environment variables
- Using a secure configuration management system
- Documenting how users should obtain and configure their own OAuth credentials
Is this intentional? If these are meant to be shared credentials for the Gemini CLI tool, please add clear documentation explaining the security implications.
|
|
||
| // Poll until operation is complete with timeout protection | ||
| const MAX_RETRIES = 30 // Maximum number of retries (60 seconds total) | ||
| let retryCount = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this timeout duration intentional? The polling could hang for up to 60 seconds (30 retries × 2 seconds). Consider:
| let retryCount = 0 | |
| const MAX_RETRIES = 15 // Maximum number of retries (30 seconds total) |
Or make it configurable through options?
| /** | ||
| * Call a Code Assist API endpoint | ||
| */ | ||
| private async callEndpoint(method: string, body: any, retryAuth: boolean = true): Promise<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The extensive use of any types here reduces type safety. Could we create proper TypeScript interfaces for the API responses? For example:
interface LoadCodeAssistResponse {
cloudaicompanionProject?: string;
allowedTiers?: Array<{ id: string; isDefault: boolean }>;
}| }) | ||
| } | ||
| } catch (error) { | ||
| throw new Error(t("common:errors.geminiCli.oauthLoadFailed", { error })) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice these error translation keys are used but I don't see them added to the locale files. Could you add the missing translations to all language files in src/i18n/locales/*/common.json?
| {t("settings:providers.geminiCli.setupLink")} | ||
| </VSCodeLink> | ||
|
|
||
| <div className="mt-3 p-3 bg-vscode-editorWidget-background border border-vscode-editorWidget-border rounded"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice UI implementation! The warning box with requirements is very helpful for users. Consider adding a visual indicator (like a green checkmark) when the OAuth file is detected at the specified path?
Related GitHub Issue
Closes: #
Roo Code Task Context (Optional)
Description
Test Procedure
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
Important
Introduces
gemini-cliprovider using OAuth, updates validation, UI components, and localization for support.gemini-cliprovider using OAuth authentication, no API key needed (gemini-cli.ts,index.ts).validate.tsto exclude API key validation forgemini-cli.GeminiClicomponent toApiOptions.tsxfor configuration.gemini-clirelated strings in multiple languages.This description was created by
for e2c37a8. You can customize this summary. It will automatically update as commits are pushed.